-
Notifications
You must be signed in to change notification settings - Fork 232
Update type hints in ORM package #7026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
605366b to
8c293c2
Compare
|
Oh boy. Misunderstood the function of |
|
Reopening, as much of this can still hold. Just not UpdateActually, |
18bc4d3 to
32a4456
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7026 +/- ##
==========================================
- Coverage 79.27% 79.26% -0.00%
==========================================
Files 566 566
Lines 43779 43805 +26
==========================================
+ Hits 34700 34717 +17
- Misses 9079 9088 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
33d343d to
1901a58
Compare
|
@edan-bainglass great to see work on typing. :-) It seems that this partly overlaps with my work in #7019 and #6704. Would be great if you could have a look and review. Especially #6704 should go in first because some of the files that you're modifying here are not actually type-checked in pre-commit. |
Okay cool. I'll have a look this week. |
c4a2d79 to
ec00bf4
Compare
ec00bf4 to
bbd8a13
Compare
|
@danielhollas honest opinion, is this PR required? I believe you stated this type of work can be automated by ruff, once some prelim work is done, which you're addressing in recent PRs. Is this accurate? |
Hah, thanks, I was going to write the same thing here but was procrastinating on it as I didn't want to be a PITA. 😅 I don't think we should merge this at this point: we should wait until we drop Python 3.9, and then convert everything over whole codebase at once with ruff. Then we'll add the resulting commit to |
Not at all. I rather close it and have the robots handle it cleanly.
Sounds like a plan 👍 We probably should look more seriously into doing 3.9 in AiiDAlab |
|
|
Most of what is done here is already accomplished by a series of recent PRs by @danielhollas. As suggested above, once we drop support for Python 3.9, we will push one commit to update all type hints across the codebase. As such, this PR is no longer necessary. Closing... |
This PR updates type hinting in the ORM module. Mostly, it's a matter of importing future annotations and updating to 3.10 standards.
Selfis added toaiida.common.typing, wheresysis used to determine from where to importSelf.Selfis introduced to improve typing for entity collections, which in turn makesCollectionTyperedundant, which is removed.Minimal formatting updates here and there.
Update
This proved to be more difficult. Reverting for now. I suspect there may be an issue with the
@classpropertydecorator that is interfering withmypy.